feat: add F0GridStack component #2994
Conversation
🔍 Visual review for your branch is published 🔍Here are the links to: |
size-limit report 📦
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the F0GridStack component, a React wrapper around the gridstack.js library that provides a draggable and resizable grid layout system. The implementation adds sophisticated features including constrained resizing with allowed sizes, simplified API, React component rendering support, imperative widget management via refs, layout change events, and widget metadata support.
Key Changes:
- Added gridstack dependency (v12.3.3) to the project
- Implemented F0GridStack component with comprehensive context providers for state management
- Created extensive test coverage for both component behavior and render provider logic
- Added Storybook documentation with interactive examples
Reviewed Changes
Copilot reviewed 14 out of 15 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added [email protected] dependency to lockfile |
| packages/react/package.json | Added gridstack dependency to package.json |
| packages/react/src/components/Utilities/F0GridStack/index.ts | Component export with experimental flag wrapper |
| packages/react/src/components/Utilities/F0GridStack/F0GridStack.tsx | Main component implementing grid with resize constraints and ref API |
| packages/react/src/components/Utilities/F0GridStack/components/types.ts | TypeScript module augmentation for gridstack types |
| packages/react/src/components/Utilities/F0GridStack/components/grid-stack-context.ts | Context definition for grid state and widget management API |
| packages/react/src/components/Utilities/F0GridStack/components/grid-stack-provider.tsx | Provider implementing grid initialization and event handling |
| packages/react/src/components/Utilities/F0GridStack/components/grid-stack-render-provider.tsx | Provider managing widget container mapping and grid lifecycle |
| packages/react/src/components/Utilities/F0GridStack/components/grid-stack-render.tsx | Component that portals React content into grid widget containers |
| packages/react/src/components/Utilities/F0GridStack/components/grid-stack-render-context.ts | Context for accessing widget container DOM elements |
| packages/react/src/components/Utilities/F0GridStack/components/grid-stack-widget-context.ts | Context providing widget metadata to rendered components |
| packages/react/src/components/Utilities/F0GridStack/components/tests/grid-stack-render-provider.test.tsx | Unit tests for WeakMap-based widget container management |
| packages/react/src/components/Utilities/F0GridStack/tests/F0GridStack.test.tsx | Comprehensive test suite covering component rendering and resize logic |
| packages/react/src/components/Utilities/F0GridStack/stories/GridStackReact.stories.tsx | Storybook examples demonstrating basic usage and ref methods |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
packages/react/src/components/Utilities/F0GridStack/components/grid-stack-provider.tsx
Show resolved
Hide resolved
| const element = document.body.querySelector<GridItemHTMLElement>( | ||
| `[gs-id="${id}"]` | ||
| ) |
There was a problem hiding this comment.
Using document.body.querySelector to find grid elements is fragile and could fail in complex DOM structures or SSR contexts. Consider using a ref to track elements or querying within a specific container scope instead of the entire document body.
| // ! gridStack is required to reinitialize the grid when the options change | ||
| [gridStack] |
There was a problem hiding this comment.
The comment states "gridStack is required to reinitialize the grid when the options change" but gridStack shouldn't need to be in the dependency array since it's already handled by the parent component's re-render logic. This may cause unnecessary re-memoization.
| // ! gridStack is required to reinitialize the grid when the options change | |
| [gridStack] | |
| // Dependency array intentionally left empty; parent re-render logic handles gridStack changes | |
| [] |
packages/react/src/components/Utilities/F0GridStack/components/grid-stack-render-context.ts
Show resolved
Hide resolved
| // Clear the WeakMap before each test | ||
| gridWidgetContainersMap.constructor.prototype.clear?.call( | ||
| gridWidgetContainersMap | ||
| ) |
There was a problem hiding this comment.
Calling .clear() on WeakMap's prototype is incorrect - WeakMaps don't have a clear method. This test cleanup logic will fail silently. WeakMaps cannot be cleared programmatically; you need to let garbage collection handle cleanup or use a different data structure for testing.
packages/react/src/components/Utilities/F0GridStack/__stories__/GridStackReact.stories.tsx
Show resolved
Hide resolved
packages/react/src/components/Utilities/F0GridStack/__stories__/GridStackReact.stories.tsx
Outdated
Show resolved
Hide resolved
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [gridStack]) |
There was a problem hiding this comment.
The eslint-disable comment masks a missing onChange dependency. While excluding onChange might be intentional to avoid re-running the effect on every callback change, this should be documented or the callback should be stabilized via useCallback in the parent.
packages/react/src/components/Utilities/F0GridStack/F0GridStack.tsx
Outdated
Show resolved
Hide resolved
…k.tsx Co-authored-by: Copilot <[email protected]>
sergiocarracedo
force-pushed
the
feat/gridstack
branch
from
706e0d6 to
1bfb521
Compare
Description
Add
F0GridStackcomponent, a wrapper over gridstask.js for react, with some specific features:Screenshots (if applicable)
Screen.Recording.2025-11-19.at.14.18.10.mov
Uploading Screen Recording 2025-11-20 at 09.15.26.mov…
Implementation details